fix(config): use ~/.devsy on macOS to avoid spaces in paths#416
Conversation
Provider init scripts run via shell and reference $PROVIDER_FOLDER. On macOS, ~/Library/Application Support/devsy contains a space which causes word-splitting in unquoted shell expansions, breaking provider initialization with "stat .../Application: no such file or directory". Use ~/.devsy consistently on macOS (matching the Linux ~/.local/share pattern in spirit) to eliminate spaces in all paths.
✅ Deploy Preview for devsydev canceled.
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe macOS path manager is updated to use Unix-style hidden home directories for configuration, data, cache, and state instead of macOS standard locations. ConfigDir and DataDir now use ChangesmacOS Path Manager Migration
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Align Linux path layout with macOS: drop XDG directory lookups in favor of ~/.devsy, ~/.cache/devsy, and /tmp/devsy-UID. This keeps provider scripts safe from word-splitting issues and simplifies cross-platform path handling.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/config/pathmanager_darwin.go (1)
22-56:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftAdd macOS migration/fallback for legacy
~/Library/*/devsydata when switching to~/.devsy/~/.cache/devsy.
pkg/config/pathmanager_darwin.gonow always computes config/data under~/.devsyand cache under~/.cache/devsy, with no fallback/migration logic for prior macOS “Library” locations. A repo-wide search only findsLibrary/Application Supportusage inpkg/ide/vscode/settings.go(unrelated), and there’s no code that copies/moves or re-reads legacy top-level directories.DEVSY_HOMEonly changes the root directory; it doesn’t migrate existing data.Implement one of: (1) first-run move/copy from legacy dirs to the new dirs, or (2) fallback reads of legacy dirs when the new dirs are empty, plus user-facing messaging/docs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/config/pathmanager_darwin.go` around lines 22 - 56, The darwinPathManager methods (ConfigDir, DataDir, CacheDir, StateDir) only return ~/.devsy and ~/.cache/devsy and must handle legacy macOS locations; implement a migration/fallback that checks legacy paths (e.g. ~/Library/Application Support/devsy for config/data, ~/Library/Caches/devsy for cache, and ~/Library/Application Support/devsy/state for state) and either (A) on first-run move or copy existing files from those legacy dirs into the new dirs and return the new path, or (B) if you prefer non-destructive behavior, return the legacy path when the new target is empty (and log/emit a user-facing message that legacy data was found and will be migrated or used). Add this logic in a single helper invoked by darwinPathManager.ConfigDir/DataDir/CacheDir/StateDir (reuse ensureDir and RepoName) and ensure the migration runs only once (persist a migrated flag or create a marker file) to avoid repeated work.
🧹 Nitpick comments (1)
pkg/config/pathmanager_darwin.go (1)
31-38: 💤 Low valueConfigDir and DataDir now return identical paths.
Both
ConfigDir()andDataDir()now return~/.devsy, meaning configuration files (e.g.,config.yaml) and data subdirectories (e.g.,contexts/,workspaces/) will coexist in the same directory. While this aligns with common CLI tool patterns on macOS (e.g., kubectl, docker) and achieves the goal of avoiding spaces, it deviates from the XDG Base Directory spec principle of separating config and data concerns.Consider whether this could cause namespace conflicts in the future if data subdirectories or config filenames overlap.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/config/pathmanager_darwin.go` around lines 31 - 38, ConfigDir() and DataDir() in darwinPathManager currently return the same path (~/.<RepoName>), risking future namespace conflicts; change darwinPathManager.DataDir to return a distinct data directory (for example using macOS convention "~/Library/Application Support/<RepoName>" or XDG_DATA_HOME fallback) instead of joining UserHome with "."+RepoName, and ensure you still call ensureDir on that new path; update references to RepoName and the ensureDir(filepath.Join(...)) call in the DataDir method to construct and create the separate data path while leaving ConfigDir unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@pkg/config/pathmanager_darwin.go`:
- Around line 22-56: The darwinPathManager methods (ConfigDir, DataDir,
CacheDir, StateDir) only return ~/.devsy and ~/.cache/devsy and must handle
legacy macOS locations; implement a migration/fallback that checks legacy paths
(e.g. ~/Library/Application Support/devsy for config/data,
~/Library/Caches/devsy for cache, and ~/Library/Application Support/devsy/state
for state) and either (A) on first-run move or copy existing files from those
legacy dirs into the new dirs and return the new path, or (B) if you prefer
non-destructive behavior, return the legacy path when the new target is empty
(and log/emit a user-facing message that legacy data was found and will be
migrated or used). Add this logic in a single helper invoked by
darwinPathManager.ConfigDir/DataDir/CacheDir/StateDir (reuse ensureDir and
RepoName) and ensure the migration runs only once (persist a migrated flag or
create a marker file) to avoid repeated work.
---
Nitpick comments:
In `@pkg/config/pathmanager_darwin.go`:
- Around line 31-38: ConfigDir() and DataDir() in darwinPathManager currently
return the same path (~/.<RepoName>), risking future namespace conflicts; change
darwinPathManager.DataDir to return a distinct data directory (for example using
macOS convention "~/Library/Application Support/<RepoName>" or XDG_DATA_HOME
fallback) instead of joining UserHome with "."+RepoName, and ensure you still
call ensureDir on that new path; update references to RepoName and the
ensureDir(filepath.Join(...)) call in the DataDir method to construct and create
the separate data path while leaving ConfigDir unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: aaac61b8-a9d3-4e1e-873d-7e25bc4ceb9a
📒 Files selected for processing (1)
pkg/config/pathmanager_darwin.go
1 participant
Summary
~/Library/Application Support/devsyto~/.devsy$PROVIDER_FOLDERwithout quotes in shell commands, which caused word-splitting on the space in "Application Support"stat /Users/.../Library/Application: no such file or directoryfollowed byinit: exit status 127~/.cache/devsy(also space-free)Summary by CodeRabbit
~/.<RepoName>) instead of the ~/Library directories.